-
Notifications
You must be signed in to change notification settings - Fork 85
Conversation
* @middleware | ||
*/ | ||
RestMiddleware.handleErrorAsJson = function (error, request, response, next) { | ||
console.log('************ handleErrorAsJson ***************'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'll remove this unnecessary log statement. Just had it there for some cheap debugging.
This is a follow-up of PR #32. General: - Refactor tests: - Namespace all tests into their separate packages (for better organization of the output), including a subspace for middleware - Rename `expect` function to `waitFor` in all calls to testAsyncMulti(), to better reflect its behavior - Copy .jshintrc into new packages json-routes: - Export RestMiddleware object for middleware packages to have a namespace to declare themselves within - Add a `JsonRoutes.middleware` to eventually replace `JsonRoutes.middleWare` (since 'middleware' is one word) - Update change log - Remove 100 character line limit in README rest-accounts-bearer-token: - Split rest-accounts-bearer-token middleware package into a middleware package that parses a standard bearer token (rest-bearer-token-parser) and one that validates an auth token and sets the request.userId to the authorized Meteor.user's ID (authenticate-meteor-user-by-token) - Attach middleware to RestMiddleware namespace, instead of adding it to every route. This allows middleware to be made available by simply adding its package. It can later be added to the middleware stack with JsonRoutes.middleware.use('path', RestMiddleware.<middlewareFunction>). - Move tests to their appropriate package simple-rest: - Add token and auth middleware in tests to pass auth tests - This could be a sign that auth tests may be better suited in the corresponding middleware packages rest-accounts-password: - Use the latest version of json-routes (1.0.3) - Add token parsing and auth middleware to the stack (currently added to all routes, which is bad - needs fix)
Hey @kahmali I'm trying to reboot getting this stuff merged and 3.0 released. Do you want to rebase and finish cleanup on this or should I take it over? |
e741877
to
0f16b4c
Compare
Hey Eric! Hope all is well. Glad to see you reviving this. I'm swamped right now, as I'm a few weeks out from an MVP launch, and I'm just trying to get all my ducks in a row with that. So, unfortunately, I won't be able to contribute much, if at all, on this for the next few weeks. I did go ahead and rebase onto the latest and clean up the few things I mentioned above. I got it situated with jshint, except for the one error I mentioned in the commit message. The other known issue mentioned in the commit still exists, and the error handling middleware doesn't actually work until that gets resolved, so this isn't ready to be merged in yet. I'm sure you'll be able to take it from here and get it working. Sorry I can't be more help with this at the moment. Restivus is now at a point where it can share middleware with simple:rest, so as soon as I have some time I hope to do my part to push both projects along. |
d2b3350
to
9616ea9
Compare
- Use connectHandlers instead of rawConnectHandlers - Add JsonRoutes.ErrorMiddleware.use for adding error handler middleware in the correct place - Remove JsonRoutes.sendError and call next(error) instead so that middleware can handle it - Export RestMiddleware as a place where packages can put their middleware functions by convention - Update connect dependency to 2.30.2 - Create rest-json-error-handler package that defines RestMiddleware.handleErrorAsJson
9f72ec6
to
53e99ed
Compare
See the discussion here for more context on this PR: #40 (comment)